Add UniFFI Kotlin bindings for payjoin-ffi - #1869
Conversation
|
@ram0verflow Thanks, I'll take a look today |
Coverage Report for CI Build 34676187573Coverage remained the same at 86.646%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
|
@ram0verflow it looks like you're hitting an unrelated probelm, that's fixed on master, you want to rebase |
caarloshenriq
left a comment
There was a problem hiding this comment.
Concept ACK.
Kotlin/JVM bindings alongside the other four is a good addition, and the layout mirrors python/csharp closely enough that it should be easy to keep in sync
|
will fix and update |
a991880 to
df8922e
Compare
chavic
left a comment
There was a problem hiding this comment.
A few more comments on the tests.
| class UriTests { | ||
| @Test | ||
| fun urlEncodedPayjoinParameter() { | ||
| val uri = "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?amount=1&pj=https://example.com?ciao" |
There was a problem hiding this comment.
Can we use a full Bitcoin URI with a percent-encoded pj value here, then assert the address, amount and complete decoded endpoint? The current fixture doesn't test encoding, and checking only the hostname would miss a dropped query. The other bindings have the same gap.
There was a problem hiding this comment.
Yes. I’ll use a percent-encoded pj and assert address, amount, and the full decoded endpoint.
|
|
||
| @Test | ||
| fun inputPairRejectsInvalidOutpoint() { | ||
| assertFailsWith<InputPairException> { |
There was a problem hiding this comment.
Should this expect InputPairException.InvalidOutPoint specifically, like C# does? I replaced deadbeef with a valid txid and this still passed because the missing UTXO information throws InvalidPsbtInput.
| import kotlin.test.Test | ||
| import kotlin.test.assertFailsWith | ||
|
|
||
| class ValidationTests { |
There was a problem hiding this comment.
Can we bring over the amount and fee-rate overflow tests from Python/C# too, including assertions on the nested error variants? Those would check Kotlin's unsigned values and error mapping across FFI.
| val state = replayReceiverEventLog(persister).state() | ||
| assertIs<ReceiveSession.Initialized>(state) | ||
| assertFalse(persister.closed) | ||
| persister.closeSession() |
There was a problem hiding this comment.
Calling persister.closeSession() directly only tests the Kotlin helper. Can we assert persister.closed after closing through the Rust session in the cancellation tests, for both sync and async? That would check the renamed callback actually gets called.
|
|
||
| # ktlint is optional; --no-format keeps generate working without it. | ||
| cargo run "${FEATURE_ARGS[@]}" --profile dev -p payjoin-ffi --bin uniffi-bindgen -- generate \ | ||
| --library "../target/$TARGET_PROFILE_DIR/$LIBNAME" \ |
There was a problem hiding this comment.
Can we use Cargo’s actual output path here and for the copy below? With CARGO_TARGET_DIR set, Cargo builds there but this still reads ../target/.
| if (txOut is JsonNull) return false | ||
| val scriptHex = txOut.jsonObject.getValue("scriptPubKey").jsonObject.getValue("hex").jsonPrimitive.content | ||
| IsScriptOwnedCallback(connection).callback(HexFormat.of().parseHex(scriptHex)) | ||
| } catch (_: Exception) { |
There was a problem hiding this comment.
Can we let RPC errors fail the test here? Returning false treats an RPC failure as “input not owned,” so a broken ownership check goes unnoticed.
| tasks.test { | ||
| useJUnitPlatform() | ||
| val libDir = layout.projectDirectory.dir("lib").asFile | ||
| val native = listOf("libpayjoin_ffi.so", "libpayjoin_ffi.dylib", "payjoin_ffi.dll") |
There was a problem hiding this comment.
Can we select the native library for the current OS? If this checkout is used on macOS and in a Linux container, both files can remain in lib/, and this always picks the .so first.
| - name: Set up nix | ||
| uses: ./.github/actions/setup-nix | ||
| - name: "Build and test" | ||
| run: nix develop .#kotlin -c bash ./payjoin-ffi/kotlin/contrib/test.sh |
There was a problem hiding this comment.
Can CI also generate with PAYJOIN_FFI_FEATURES= and compile the resulting Kotlin bindings? This job only exercises _test-utils, so the documented production configuration isn’t covered.
Generate with in-tree uniffi-bindgen --language kotlin, siloed like Python (no extra cargo feature). Rename protocol close to closeSession in Kotlin only so AutoCloseable still works. Pin the Gradle 9.1.0 wrapper with distributionSha256Sum; validateDistributionUrl only checks the download URL. Add a kotlin nix dev shell (JDK 21, MSRV, bitcoind) and run CI inside it, matching the other language bindings. Unit tests cover URIs, persistence, cancel, and validation.
Drive a full BIP77 round trip against the in-process directory, OHTTP relay, and bitcoind, mirroring the Python suite's RPC sequence and assertions. TestHttp trusts only the self-signed rcgen directory cert so HTTPS through the relay succeeds in the JVM. Parse bitcoind RPC responses with kotlinx-serialization-json.
Honor CARGO_TARGET_DIR, load the OS-native library, and assert percent-encoded URIs, overflow variants, and persister close via Rust. Co-authored-by: Cursor <cursoragent@cursor.com>
df8922e to
64a8082
Compare
Pull Request Checklist
Please confirm the following before requesting review:
AI
in the body of this PR.
What
UniFFI Kotlin/JVM bindings for
payjoin-ffi, plus a BIP77 v2 round-tripintegration test.
Layout matches python/csharp:
payjoin-ffi/kotlin/scripts/generate_bindings.shpayjoin-ffi/kotlin/contrib/test.shPAYJOIN_FFI_FEATURES(default_test-utils; empty string is passedthrough) and
PAYJOIN_FFI_PROFILE(defaultdev→target/debug)Generated sources are not committed (gitignored under
kotlin/src/main/kotlin/org/). Generate is a build step.CI runs inside
nix develop .#kotlin(JDK 21, MSRV, nixpkgs bitcoind),matching csharp/python/dart.
Unit tests (15) cover URIs, persistence, cancel, and validation — the
same count as the Python unit suite. The second commit adds the v2
integration test (16 Gradle tests on HEAD).
Why
[bindings.kotlin.rename]Generated UniFFI objects implement
DisposableandAutoCloseable, soclose()drops the Rust handle. Payjoin also exports protocolclose()onReceiverPendingFallback,SenderPendingFallback, and the four JSONsession persister traits. Kotlin cannot overload on return type.
The table is Kotlin-only. It maps those protocol methods to
closeSession(). No Rust API change. Other language bindings keepclose.Integration test
IntegrationTests.ktdrives a full BIP77 v2 round trip, sender andreceiver, against the in-process directory, OHTTP relay, and a real
bitcoind. Under the kotlin nix shell,
BITCOIND_EXEcomes from nixpkgsand
BITCOIND_SKIP_DOWNLOAD=1. Outside the shell,corepc-nodemaydownload Bitcoin Core into a temp regtest dir.
On Linux,
v2ToV2Payjoincompletes in about 4s.TestHttptrusts only the directory's rcgen cert and proxies through theOHTTP relay.
Wrapper pin
Gradle 9.1.0 wrapper is pinned with
distributionSha256Sum.validateDistributionUrlonly checks the URL.Release / publish
Not published anywhere. No pack, Maven, signing, or attestation jobs.
The workflow does not trigger on
payjoin-kotlin-*tags. python.yml'stag trigger runs wheel build, PyPI publish, and GitHub release assets.
Until Kotlin has something to publish, a tag trigger would only re-run
tests.
Follow-ups (out of this PR)
.ktDisclosure
This PR was written with Cursor Cloud Agent (Grok 4.6).
shell, CI workflow, unit tests, and the v2 integration test were
AI-generated and then hand-reviewed against the Python suite and a
known-good round trip (~4s).
Disclosure: co-authored by Cursor Grok 4.6